-
Notifications
You must be signed in to change notification settings - Fork 49
Move away from simple strat for SM keyspace #4727
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
SimpleStrategy is not recommended for the production environments. It also does not support tablets, which are the way towards scylla is moving (in some distant time, vnodes might even be deprecated). Because of that, it also runs into unexpected problems like #4555. Because of those reasons, we should switch from keeping SM data in SimpleStrategy to NetworkTopologyStrategy keyspace by default. For the default, single local node SM DB cluster, both strategies result in the single node containing all of SM data. For single DC SM DB cluster they are also equivalent. On the other hand, for multi DC SM DB cluster (really not recommended...), NetworkTopologyStrategy results in keeping replication_factor replicas per DC and not per cluster. This is theoretically problematic, but such setup is also problematic for SimpleStrategy, which would still replicate the data in all DCs, so it's not a new problem. Moreover, this change only defines the default SM keyspace creation. In case user has some non-default (perhaps not recommended) SM DB setup, they can always create SM keyspace manually before starting SM server to configure it to their liking. Refs #4555
…n tests Because of #4555, but also because it's better to test recommended keyspace replication strategy, we need to replace SimpleStrategy with NetworkTopologyStrategy in tests wherever possible. In case test is aiming to run against SimpleStrategy specifically, it needs to be adjusted so that it ensures that keyspace with SimpleStrategy does not use tablets. Refs #4555
ba6360e to
2e0a379
Compare
VAveryanov8
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems reasonable to me!
I'm curios, how how will it behave with replace manager procedure in siren, e.g. when the scylla-manager schema is restored from the self-backup?
So this PR only changes how SM keyspace is created from scratch. In terms of siren self-backup, if SM keyspace in the backup was using SimpleStrategy, during the self-restore procedure SM would create new, tmp SM keyspace with NetworkTopologyStrategy from scratch, restore SM keyspace from the backup and switch back to using the one from the backup. |
|
@paszkow does the approach presented in this PR make sense to you? |
paszkow
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moving to a NetworkTopologyStrategy is definitely better, but it raises a few questions:
- How big is the keyspace in production? We might want to tweak tablet options for it.
- When is it created?
- With the
rf_rack_valid_keyspacesoption, it's yet another non-user keyspace that a user will have to alter while decommissioning a rack. We need to keep that in mind.
\cc @bhalevy
| } | ||
|
|
||
| const createKeyspaceStmt = "CREATE KEYSPACE {{.Keyspace}} WITH replication = {'class': 'SimpleStrategy', 'replication_factor': {{.ReplicationFactor}}}" | ||
| const createKeyspaceStmt = "CREATE KEYSPACE {{.Keyspace}} WITH replication = {'class': 'NetworkTopologyStrategy', 'replication_factor': {{.ReplicationFactor}}}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you know how big this table is in production? When tablets are enabled, it will create 10 tablets/shard by default. Maybe we should start small and let a load balancer split it when needed. \cc @bhalevy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See this comment for more context.
| # local_dc: | ||
| # | ||
| # Keyspace for management data. | ||
| # It will be created automatically on ScyllaDB Manager server startup if it does not already exist. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the manager server created once a cluster is fully constructed? By default RF=3 is used. That requires 3 nodes/racks to already exist. Otherwise, the keyspace will not be created.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See this comment for more context.
|
Let me put some more context here. In terms of SM backend specification, in theory, it's possible to use any scylla cluster as SM backend, but the default orchestration (also the one used in siren and operator) is that we use a single node scylla cluster located on SM VM. This cluster runs on a single shard and is limited to 500M memory. It also runs in developer mode. This should demonstrate that we expect small amount of data to be kept there - only SM task definitions and their execution history/progress. Having said that, we've seen some custom, manual deployments where SM cluster and user cluster are the same ones, but in such case, it's still possible to create SM keyspace manually to specify desired replication and such. In terms of this keyspace creation, when SM server is started, it checks if SM keyspace exists. If it does, it uses it for storing its data. If it doesn't exist, it creates it (previously with simple strategy, after this PR with network topology strategy). So the changes made in this PR won't affect existing deployments, only the new ones. Moreover, IIUC, moving from simple strategy to network topology strategy for a single node cluster shouldn't have a lot of impact. The same goes for initial tablet count, as all data is stored on a single node anyway. In terms of @paszkow does this answers your concerns (or perhaps it created more of them)? |
Alternative approach to #4555.
While the other PR focused on keeping SimpleStrategy and working around its limitations, this PR goes into direction of using NetworkTopologyStrategy instead.
The reasoning for that is:
More details in the commit messages.
Merged commit messages
SimpleStrategy is not recommended for the production environments.
It also does not support tablets, which are the way towards scylla
is moving (in some distant time, vnodes might even be deprecated).
Because of that, it also runs into unexpected problems like
#4555.
Because of those reasons, we should switch from keeping SM data in
SimpleStrategy to NetworkTopologyStrategy keyspace by default.
For the default, single local node SM DB cluster, both strategies
result in the single node containing all of SM data. For single DC
SM DB cluster they are also equivalent. On the other hand, for multi
DC SM DB cluster (really not recommended...), NetworkTopologyStrategy
results in keeping replication_factor replicas per DC and not per cluster.
This is theoretically problematic, but such setup is also problematic for
SimpleStrategy, which would still replicate the data in all DCs, so it's
not a new problem. Moreover, this change only defines the default SM
keyspace creation. In case user has some non-default (perhaps not recommended)
SM DB setup, they can always create SM keyspace manually before starting
SM server to configure it to their liking.
Because of #4555, but also because
it's better to test recommended keyspace replication strategy, we need to replace
SimpleStrategy with NetworkTopologyStrategy in tests wherever possible.
In case test is aiming to run against SimpleStrategy specifically, it needs to be
adjusted so that it ensures that keyspace with SimpleStrategy does not use tablets.